Skip to content

Add execution level tagging#44

Merged
nprizal merged 2 commits intomainfrom
pie-3500-add-execution-level-tagging-for-pytest-test-collector
Mar 27, 2025
Merged

Add execution level tagging#44
nprizal merged 2 commits intomainfrom
pie-3500-add-execution-level-tagging-for-pytest-test-collector

Conversation

@nprizal
Copy link
Contributor

@nprizal nprizal commented Mar 26, 2025 •
edited
Loading

Add execution level tagging using execution_tag custom marker. The test collector will send all tags marked on each test to Buildkite Test Engine. See pytest's custom markers documentation.

tagging individual tests using decorator:

import pytest

@pytest.mark.execution_tag("key", "value")
@pytest.mark.execution_tag("another.key", "value")
def test_add():
     assert 1 + 1 == 2

tagging all tests in a test class using decorator:

import pytest

@pytest.mark.execution_tag("key", "value")
@pytest.mark.execution_tag("another.key", "value")
class TestSubstract:
    def test_positive(self):
        assert 5 - 4 == 1

    def test_negative(self):
        assert 2 - 3 = -1

tagging all tests in a module using pytestmark helper

import pytest

pytestmark=[
    pytest.mark.execution_tag("key", "value"),
    pytest.mark.execution_tag("another.key", "value")
]

def test_add():
     assert 1 + 1 == 2

tagging tests from a hook:

def pytest_runtest_setup(item):
    item.add_marker(pytest.mark.execution_tag("key", "value"))
    item.add_marker(pytest.mark.execution_tag("another.key", "value"))

@nprizal nprizal force-pushed the pie-3500-add-execution-level-tagging-for-pytest-test-collector branch from e6c2e22 to 2ea75e7 Compare March 27, 2025 01:36
@nprizal nprizal changed the title Add execution level tagging for pytest test collector Add execution level tagging Mar 27, 2025
@nprizal nprizal self-assigned this Mar 27, 2025
@nprizal nprizal requested review from a team and zhming0 March 27, 2025 01:50
@nprizal nprizal marked this pull request as ready for review March 27, 2025 01:52
nprizal
nprizal commented Mar 27, 2025
View reviewed changes
src/buildkite_test_collector/pytest_plugin/buildkite_plugin.py
def pytest_runtest_logreport(self, report):
"""pytest_runtest_logreport hook callback"""
if report.when != 'call':
if report.when != 'teardown':
Copy link
Contributor Author

@nprizal nprizal Mar 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Defer the execution colection after the test teardown to allow adding execution_tag during the call hook.

Copy link
Contributor

@zhming0 zhming0 Mar 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite understand this bit. I thought logreport is always after the teardown hook? What does this if condition do?

@nprizal nprizal force-pushed the pie-3500-add-execution-level-tagging-for-pytest-test-collector branch from 2ea75e7 to 234266e Compare March 27, 2025 02:10
zhming0
zhming0 previously approved these changes Mar 27, 2025
View reviewed changes
Copy link
Contributor

@zhming0 zhming0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, looks great 🙂

src/buildkite_test_collector/collector/payload.py Outdated
Comment on lines 139 to 141
def tag_execution(self, key, val) -> 'TestData':
"""Set tag to test execution"""
self.tags[key] = val
Copy link
Contributor

@zhming0 zhming0 Mar 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not a blocker, I wonder what will happen if user put non-string key value here.

Copy link
Contributor Author

@nprizal nprizal Mar 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah! I wasn't aware that tag only support string. I'll handle non-string scenario.

src/buildkite_test_collector/pytest_plugin/buildkite_plugin.py
def pytest_runtest_logreport(self, report):
"""pytest_runtest_logreport hook callback"""
if report.when != 'call':
if report.when != 'teardown':
Copy link
Contributor

@zhming0 zhming0 Mar 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite understand this bit. I thought logreport is always after the teardown hook? What does this if condition do?

@nprizal
Copy link
Contributor Author

nprizal commented Mar 27, 2025

I don't quite understand this bit. I thought logreport is always after the teardown hook? What does this if condition do?

The logreport hook is called after setup, call, and teardown. Previously, the test is reported (i.e. appended to the payload) on the logreport hook following a call. Because we are allowing user to add tag during a hook (most likely setup/call, we need to collect that tag at the end, which is when the test is teardown.

@zhming0
Copy link
Contributor

zhming0 commented Mar 27, 2025

@nprizal please give me a sec, I will have another read at this.

@zhming0 zhming0 dismissed their stale review March 27, 2025 04:51

Taking another look, gaining more pytest understanding

zhming0
zhming0 approved these changes Mar 27, 2025
View reviewed changes
Copy link
Contributor

@zhming0 zhming0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand it fully now. Let's 🚀 .

I don't think we need an abstraction on top of the marker just yet.

@nprizal
Copy link
Contributor Author

nprizal commented Mar 27, 2025

I pushed changes to validate the type of key and value and raise error when it's not a string (to follow the behavior of ruby collector)

@nprizal nprizal requested a review from zhming0 March 27, 2025 05:35
zhming0
zhming0 approved these changes Mar 27, 2025
View reviewed changes
Copy link
Contributor

@zhming0 zhming0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@nprizal nprizal merged commit 2fe6710 into main Mar 27, 2025
11 checks passed
@nprizal nprizal deleted the pie-3500-add-execution-level-tagging-for-pytest-test-collector branch March 27, 2025 05:39
This was referenced Mar 27, 2025
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zhming0 zhming0 zhming0 approved these changes

Assignees

@nprizal nprizal

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nprizal @zhming0